Skip to content

feat(dashboards): Add DashboardRevision model and migration#112929

Merged
skaasten merged 6 commits intomasterfrom
skaasten/feat/dashboard-revision-migration
Apr 14, 2026
Merged

feat(dashboards): Add DashboardRevision model and migration#112929
skaasten merged 6 commits intomasterfrom
skaasten/feat/dashboard-revision-migration

Conversation

@skaasten
Copy link
Copy Markdown
Contributor

@skaasten skaasten commented Apr 14, 2026

Add the DashboardRevision model and its migration as the storage layer for the Dashboard Revisions feature, which will give users the ability to revert a dashboard to a previous version.

Each revision captures a point-in-time snapshot of a dashboard at the time of an edit, including:

  • snapshot — full JSON representation of the dashboard state (text-backed field)
  • snapshot_schema_version — integer version of the snapshot schema, so future schema changes can be handled during restore without a full backfill
  • title — dashboard title at the time of the revision
  • source — what triggered the save (e.g. "edit")
  • created_by_id — the user who made the change (nullable, SET_NULL on user delete)
  • Foreign key to Dashboard (CASCADE)

A composite index on (dashboard_id, date_added DESC) supports the primary query for fetching a dashboard's history, ordered newest-first.

Refs DAIN-1515

Add DashboardRevision model to store snapshots of dashboard state at
the time of each edit. Each revision records the dashboard title,
source action, a JSON snapshot of the full dashboard state, schema
version, and the user who made the change.

Includes a composite index on (dashboard_id, date_added DESC) to
support efficient per-dashboard history lookups.

Refs DAIN-1515
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 14, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 14, 2026
Organization is already reachable via dashboard.organization_id.
The direct FK was unnecessary denormalization with no query benefit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@skaasten skaasten marked this pull request as ready for review April 14, 2026 15:54
@skaasten skaasten requested review from a team as code owners April 14, 2026 15:54
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

This PR has a migration; here is the generated SQL for src/sentry/migrations/1067_add_dashboard_revision_model.py

for 1067_add_dashboard_revision_model in sentry

--
-- Create model DashboardRevision
--
CREATE TABLE "sentry_dashboardrevision" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "created_by_id" bigint NULL, "date_updated" timestamp with time zone NOT NULL, "date_added" timestamp with time zone NOT NULL, "title" varchar(255) NOT NULL, "source" varchar(32) NOT NULL, "snapshot" text NOT NULL, "snapshot_schema_version" integer NOT NULL, "dashboard_id" bigint NOT NULL);
ALTER TABLE "sentry_dashboardrevision" ADD CONSTRAINT "sentry_dashboardrevi_dashboard_id_7b943d04_fk_sentry_da" FOREIGN KEY ("dashboard_id") REFERENCES "sentry_dashboard" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_dashboardrevision" VALIDATE CONSTRAINT "sentry_dashboardrevi_dashboard_id_7b943d04_fk_sentry_da";
CREATE INDEX CONCURRENTLY "sentry_dashboardrevision_created_by_id_f69f0d9a" ON "sentry_dashboardrevision" ("created_by_id");
CREATE INDEX CONCURRENTLY "sentry_dashboardrevision_dashboard_id_7b943d04" ON "sentry_dashboardrevision" ("dashboard_id");
CREATE INDEX CONCURRENTLY "sentry_dashrev_dash_date_idx" ON "sentry_dashboardrevision" ("dashboard_id", "date_added" DESC);

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 14, 2026

Backend Test Failures

Failures on e3c6104 in this run:

tests/sentry/backup/test_exhaustive.py::ExhaustiveTests::test_exhaustive_dirty_pkslog
tests/sentry/backup/test_exhaustive.py:39: in test_exhaustive_dirty_pks
    verify_models_in_output(expected_models, actual)
tests/sentry/backup/__init__.py:39: in verify_models_in_output
    raise AssertionError(
E   AssertionError: Some `expected_models` entries were not found: ['sentry.dashboardrevision']
E   
E               If you are seeing this in CI, it means that this test produced an `export.json` backup
E               file that was missing the above models. This check is in place to ensure that ALL models
E               of a certain category are covered by this particular test - by omitting a certain kind
E               of model from the backup output entirely, we end up in a situation where backing up the
E               model in question to JSON is untested.
E   
E               To fix this, you'll need to modify the body of the test to add at least one of these
E               models to the database before exporting. The process for doing so is test-specific, but
E               if the test body contains a fixture factory like `self.create_exhaustive_...`, that
E               function will be a good place to start. If it does not, you can just write the model to
E               the database at the appropriate point in the test: `MyNewModel.objects.create(...)`.
tests/sentry/backup/test_exhaustive.py::ExhaustiveTests::test_uniquenesslog
tests/sentry/backup/test_exhaustive.py:63: in test_uniqueness
    verify_models_in_output(expected_models, actual)
tests/sentry/backup/__init__.py:39: in verify_models_in_output
    raise AssertionError(
E   AssertionError: Some `expected_models` entries were not found: ['sentry.dashboardrevision']
E   
E               If you are seeing this in CI, it means that this test produced an `export.json` backup
E               file that was missing the above models. This check is in place to ensure that ALL models
E               of a certain category are covered by this particular test - by omitting a certain kind
E               of model from the backup output entirely, we end up in a situation where backing up the
E               model in question to JSON is untested.
E   
E               To fix this, you'll need to modify the body of the test to add at least one of these
E               models to the database before exporting. The process for doing so is test-specific, but
E               if the test body contains a fixture factory like `self.create_exhaustive_...`, that
E               function will be a good place to start. If it does not, you can just write the model to
E               the database at the appropriate point in the test: `MyNewModel.objects.create(...)`.
tests/sentry/backup/test_exports.py::ScopingTests::test_global_export_scopinglog
tests/sentry/backup/test_exports.py:399: in test_global_export_scoping
    self.verify_model_inclusion(unencrypted, ExportScope.Global)
tests/sentry/backup/test_exports.py:182: in verify_model_inclusion
    raise AssertionError(
E   AssertionError: The following models were not included in the export: ${<class 'sentry.models.dashboard.DashboardRevision'>}; this is despite it being included in at least one of the following relocation scopes: {<RelocationScope.Config: 4>, <RelocationScope.Organization: 3>, <RelocationScope.User: 2>, <RelocationScope.Global: 5>}
tests/sentry/backup/test_exports.py::ScopingTests::test_organization_export_scopinglog
tests/sentry/backup/test_exports.py:263: in test_organization_export_scoping
    self.verify_model_inclusion(unencrypted, ExportScope.Organization)
tests/sentry/backup/test_exports.py:182: in verify_model_inclusion
    raise AssertionError(
E   AssertionError: The following models were not included in the export: ${<class 'sentry.models.dashboard.DashboardRevision'>}; this is despite it being included in at least one of the following relocation scopes: {<RelocationScope.Organization: 3>, <RelocationScope.User: 2>}
tests/sentry/backup/test_imports.py::ScopingTests::test_global_import_scopinglog
tests/sentry/backup/test_imports.py:885: in test_global_import_scoping
    self.verify_model_inclusion(ImportScope.Global)
tests/sentry/backup/test_imports.py:813: in verify_model_inclusion
    assert model.objects.count() > 0
E   AssertionError: assert 0 > 0
E    +  where 0 = <bound method QuerySet.count of <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0>>()
E    +    where <bound method QuerySet.count of <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0>> = <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0>.count
E    +      where <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0> = <class 'sentry.models.dashboard.DashboardRevision'>.objects
tests/sentry/backup/test_imports.py::ScopingTests::test_organization_import_scopinglog
tests/sentry/backup/test_imports.py:845: in test_organization_import_scoping
    self.verify_model_inclusion(ImportScope.Organization)
tests/sentry/backup/test_imports.py:813: in verify_model_inclusion
    assert model.objects.count() > 0
E   AssertionError: assert 0 > 0
E    +  where 0 = <bound method QuerySet.count of <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0>>()
E    +    where <bound method QuerySet.count of <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0>> = <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0>.count
E    +      where <sentry.db.models.manager.base.BaseManager object at 0x7f876ddb2cf0> = <class 'sentry.models.dashboard.DashboardRevision'>.objects
tests/sentry/backup/test_coverage.py::test_all_eligible_organization_scoped_models_tested_for_user_mergelog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/backup/test_coverage.py:135: in test_all_eligible_organization_scoped_models_tested_for_user_merge
    assert not {str(u) for u in untested}, (
E   AssertionError: The aforementioned models are not covered in the `ORG_MEMBER_MERGE` backup tests; please go to `tests/sentry/models/test_user.py::UserMergeToTest` and make sure at least one test in the suite contains covers each of the missing models.
E   assert not {'sentry.dashboardrevision'}

…frastructure

New models with RelocationScope.Organization that reference a user require
three additions: fixture creation in create_exhaustive_organization, inclusion
in the user merge model_list, and coverage in the expect_models decorators.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly lgtm, just a few comments



@cell_silo_model
class DashboardRevision(Model):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use DefaultFieldsModel here to get date_added and date_updated for free

title = models.CharField(max_length=255)
source = models.CharField(max_length=32, default="edit")
snapshot: models.Field[dict[str, Any], dict[str, Any]] = JSONField(default=dict)
snapshot_schema_version = models.IntegerField(db_default=1)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Wondering if it makes sense to not have a default here, so that we're always explicit about which schema we're saving



@cell_silo_model
class DashboardRevision(Model):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we automatically delete these once they're old enough, or will we keep them around until the Dashboard is deleted?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will have a limit per dashboard. New revisions above that will delete the oldest one. We'll start with 10 and adjust if necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use "source" to differentiate edits vs ai edits.

Gets date_added (auto_now_add) and date_updated (auto_now) for free,
removes the manually declared date_added field.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Force callers to be explicit about which schema version they are
saving rather than silently defaulting to 1.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on b1e6853 in this run:

tests/sentry/backup/test_exhaustive.py::ExhaustiveTests::test_exhaustive_dirty_pkslog
tests/sentry/backup/test_exhaustive.py:38: in test_exhaustive_dirty_pks
    actual = self.import_export_then_validate(self._testMethodName, reset_pks=False)
src/sentry/testutils/helpers/backups.py:971: in import_export_then_validate
    return import_export_then_validate(out_name, reset_pks=reset_pks)
src/sentry/testutils/helpers/backups.py:349: in import_export_then_validate
    raise ValidationError(res)
E   sentry.testutils.helpers.backups.ValidationError: ComparatorFinding(
E       kind: UnequalJSON,
E       on: InstanceID(model: 'sentry.dashboardrevision', ordinal: 1),
E       left_pk: 2,
E       right_pk: 1,
E       reason: 
E       --- 
E   
E       +++ 
E   
E       @@ -1,8 +1,8 @@
E   
E        {
E       -  "date_added": "2026-04-14T17:20:10.600Z",
E       -  "date_updated": "2026-04-14T17:20:10.600Z",
E       +  "date_added": "2026-04-14T17:20:14.235Z",
E       +  "date_updated": "2026-04-14T17:20:14.235Z",
E          "snapshot": {},
E          "snapshot_schema_version": 1,
E          "source": "edit",
E          "title": "Dashboard 1 for test-org"
E        }
E   )

The exhaustive backup test exports and re-imports data, then compares
JSON field-by-field. The auto_now/auto_now_add fields date_added and
date_updated are re-stamped at import time, causing a spurious
UnequalJSON ComparatorFinding. Register a DateUpdatedComparator so the
backup infrastructure treats these fields as "may be later" rather than
requiring exact equality.

Co-Authored-By: Claude Sonnet 4 <noreply@example.com>
Copy link
Copy Markdown
Contributor

@DominikB2014 DominikB2014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! just one question about one of the fields

Comment on lines +434 to +435
created_by_id = HybridCloudForeignKey(
"sentry.User", db_index=True, null=True, on_delete="SET_NULL"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this is how we know who made the revision, will we just assume null created_by_id is ai? or will created_by_id just be the id of the user who prompted the ai?

If it's the later, it might be useful to know wether the revision is ai created or user created.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be the user that initiated/confirmed the changes made by the agent (since it doesn't save without confirming).

@skaasten skaasten merged commit 4f234a5 into master Apr 14, 2026
78 checks passed
@skaasten skaasten deleted the skaasten/feat/dashboard-revision-migration branch April 14, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants